Additional variable expansion features, some support for Windows #493
Conversation
Default expansion is more strict now (undefined vs. empty). The supported set is closer to what is available in docker-compose See: https://docs.docker.com/compose/environment-variables/env-file/
Single quoted strings do not expand variables in bash. I think the libary was missing a simple way to selectively disable variable expansion.
Error results in no quote.
I corrected flake8 errors in this commit. test_parser.py had a lot of long lines, formatted it with black. Importing sh is done in try/except. Although the package is available for Windows, it is failing on import. Moved previous manual tests into proper pytest. Utility for simulating Windows os.environment behavior moved to a separate file.
This commit brings back the old interface for `resolve_variables`. I realized it might be actually used by the clients. I made _resolve_bindings private. Created resolve_variable as a result of refactoring. It be as useful as resolve_variables for the clients.
|
I realized that |
|
Closing this one as I opened it from my main and now I want to clean it up. For reference I have it on branch here: https://github.com/Bajron/python-dotenv/tree/expansions-improvements-windows I wanted to follow up these changes with bash-like behavior in terms of string resolution (e.g. "foo"bar'baz' makes a string foobarbaz; it is useful if you want to disable expansions for a part of the string): https://github.com/Bajron/python-dotenv/tree/string-expansion I understand these might be features that only I want and could be too risky to introduce in a stable package. No worries, I figured out that I can simply use my fork as a dependency with |
It is so cool this package exist :) Found it accidentally and it seems exactly what I need in my project. While reviewing the features I missed some, so I implemented it. During the implementation found some problems, and the result is this PR.
New features:
Support additional basic expansions.
In the current version we have only "default if none or empty" (
${X:-a}), I think we could add "required" (${X:?a}) and "replace"(${X:+a}). We can also distinguish "not set" from "not set or empty".My main motivation was to be closer to what
docker-composesupports.The implementation was quite straightforward in the current design. I modified the regex, and added a class to support detected actions.
Provide an easy way to disable expansion for some variables.
I thought maybe escaping should do that, but it does something else (resulted in some tests documenting the current behavior)
The other option would be to use single quotes. This is what I added.
I modified
DotEnv.dict()and what we keep inBinding. Now we know what quotes were used in the binding, and then we use that to conditionally resolve variables. The new behavior is enabled by a new parameter to the DotEnv. By default it works as before.I needed to update the parser test and it was fine until I run
flake8. Just formatting it withblackseemed like a reasonable option. The code might be not easy to compare, but definitely much easier to read now.During testing I found out, that
shdoes not seem to be supported on Windows, so I added some code to handle the errors. As a result a few tests are skipped on Windows.Some tests for expansions had a very confusing results. It appeared that it was a quite specific behaviour of
os.environon Windows. Every key is converted to uppercase.This was not expected in our testcases, so for now I used uppercase names in the expansion tests, and for other tests I provided
as_envwhich introduces this behavior for our expected values.This behavior can also be a problem for the
override=Falsesetting. I introduced additional check for Windows:main...Bajron:python-dotenv:main#diff-7d2db4b09c013c58401021bf63dbd3236144a8babb96a6ca9d7cf4fd8a2ed90fR107
I have added a note about this behavior in the README file.
Please forgive me a big PR containing several changes. If it's too much I could rework it.